fs-back: better error handling in fs-backend

Currently most of the error checking in fs-backend is done by the use
of asserts that would terminate the daemon in case of a single error
on a single request.  This patch replaces the asserts with debugging
messages and terminates the connection on which the error occurred.
With this patch applied I was able to complete successfully over 1000
live migrations with stubdoms.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/tools/fs-back/fs-backend.c b/tools/fs-back/fs-backend.c
index 3b68d7658e01c4f6e8148df7440296fa63978d7d..648b63b4712a67c59026261f015a00483a17586b 100644 (file)
--- a/tools/fs-back/fs-backend.c
+++ b/tools/fs-back/fs-backend.c
@@ -144,7 +144,8 @@ moretodo:
         xc_evtchn_notify(mount->evth, mount->local_evtchn);
 }
 
-static void terminate_mount_request(struct fs_mount *mount) {
+void terminate_mount_request(struct fs_mount *mount)
+{
     int count = 0, i;
 
     FS_DEBUG("terminate_mount_request %s\n", mount->frontend);
@@ -158,7 +159,13 @@ static void terminate_mount_request(struct fs_mount *mount) {
         }
     mount->nr_entries = count;
 
-    while (!xenbus_frontend_state_changed(mount, STATE_CLOSING));
+    /* wait for the frontend to shut down but don't wait more than 3
+     * seconds */
+    i = 0;
+    while (!xenbus_frontend_state_changed(mount, STATE_CLOSING) && i < 3) {
+        sleep(1);
+        i++;
+    }
     xenbus_write_backend_state(mount, STATE_CLOSED);
 
     xc_gnttab_munmap(mount->gnth, mount->ring.sring, mount->shared_ring_size);
@@ -183,7 +190,7 @@ static void handle_connection(int frontend_dom_id, int export_id, char *frontend
 {
     struct fs_mount *mount;
     struct fs_export *export;
-    struct fsif_sring *sring;
+    struct fsif_sring *sring = NULL;
     uint32_t dom_ids[MAX_RING_SIZE];
     int i;
 
@@ -204,24 +211,38 @@ static void handle_connection(int frontend_dom_id, int export_id, char *frontend
     }
 
     mount = (struct fs_mount*)malloc(sizeof(struct fs_mount));
+    memset(mount, 0, sizeof(struct fs_mount));
     mount->dom_id = frontend_dom_id;
     mount->export = export;
     mount->mount_id = mount_id++;
-    xenbus_read_mount_request(mount, frontend);
+    if (xenbus_read_mount_request(mount, frontend) < 0)
+        goto error;
     FS_DEBUG("Frontend found at: %s (gref=%d, evtchn=%d)\n", 
             mount->frontend, mount->grefs[0], mount->remote_evtchn);
-    xenbus_write_backend_node(mount);
+    if (!xenbus_write_backend_node(mount)) {
+        FS_DEBUG("ERROR: failed to write backend node on xenbus\n");
+        goto error;
+    }
     mount->evth = -1;
     mount->evth = xc_evtchn_open(); 
-    assert(mount->evth != -1);
+    if (mount->evth < 0) {
+        FS_DEBUG("ERROR: Couldn't open evtchn!\n");
+        goto error;
+    }
     mount->local_evtchn = -1;
     mount->local_evtchn = xc_evtchn_bind_interdomain(mount->evth, 
                                                      mount->dom_id, 
                                                      mount->remote_evtchn);
-    assert(mount->local_evtchn != -1);
+    if (mount->local_evtchn < 0) {
+        FS_DEBUG("ERROR: Couldn't bind evtchn!\n");
+        goto error;
+    }
     mount->gnth = -1;
     mount->gnth = xc_gnttab_open(); 
-    assert(mount->gnth != -1);
+    if (mount->gnth < 0) {
+        FS_DEBUG("ERROR: Couldn't open gnttab!\n");
+        goto error;
+    }
     for(i=0; i<mount->shared_ring_size; i++)
         dom_ids[i] = mount->dom_id;
     sring = xc_gnttab_map_grant_refs(mount->gnth,
@@ -230,16 +251,40 @@ static void handle_connection(int frontend_dom_id, int export_id, char *frontend
                                      mount->grefs,
                                      PROT_READ | PROT_WRITE);
 
+    if (!sring) {
+        FS_DEBUG("ERROR: Couldn't amp grant refs!\n");
+        goto error;
+    }
+
     BACK_RING_INIT(&mount->ring, sring, mount->shared_ring_size * XC_PAGE_SIZE);
     mount->nr_entries = mount->ring.nr_ents; 
     for (i = 0; i < MAX_FDS; i++)
         mount->fds[i] = -1;
 
     LIST_INSERT_HEAD(&mount_requests_head, mount, entries);
-    xenbus_watch_frontend_state(mount);
-    xenbus_write_backend_state(mount, STATE_READY);
-    
+    if (!xenbus_watch_frontend_state(mount)) {
+        FS_DEBUG("ERROR: failed to watch frontend state on xenbus\n");
+        goto error;
+    }
+    if (!xenbus_write_backend_state(mount, STATE_READY)) {
+        FS_DEBUG("ERROR: failed to write backend state to xenbus\n");
+        goto error;
+    }
+
     allocate_request_array(mount);
+
+    return;
+
+error:
+    xenbus_write_backend_state(mount, STATE_CLOSED);
+    if (sring)
+        xc_gnttab_munmap(mount->gnth, mount->ring.sring, mount->shared_ring_size);
+    if (mount->gnth > 0)
+        xc_gnttab_close(mount->gnth);
+    if (mount->local_evtchn > 0)
+        xc_evtchn_unbind(mount->evth, mount->local_evtchn);
+    if (mount->evth > 0)
+        xc_evtchn_close(mount->evth);
 }
 
 static void await_connections(void)

diff --git a/tools/fs-back/fs-backend.h b/tools/fs-back/fs-backend.h
index 504569027e4c1cf3ecfe6362a75404c4d1d0b5ad..9399841a57763fbfe13c04072f35214690db2754 100644 (file)
--- a/tools/fs-back/fs-backend.h
+++ b/tools/fs-back/fs-backend.h
@@ -56,6 +56,7 @@ struct fs_mount
     LIST_ENTRY(fs_mount) entries;
 };
 
+void terminate_mount_request(struct fs_mount *mount);
 
 /* Handle to XenStore driver */
 extern struct xs_handle *xsh;
@@ -63,12 +64,12 @@ extern struct xs_handle *xsh;
 bool xenbus_create_request_node(void);
 int xenbus_register_export(struct fs_export *export);
 int xenbus_get_watch_fd(void);
-void xenbus_read_mount_request(struct fs_mount *mount, char *frontend);
-void xenbus_write_backend_node(struct fs_mount *mount);
-void xenbus_write_backend_state(struct fs_mount *mount, const char *state);
+int xenbus_read_mount_request(struct fs_mount *mount, char *frontend);
+bool xenbus_write_backend_node(struct fs_mount *mount);
+bool xenbus_write_backend_state(struct fs_mount *mount, const char *state);
 int xenbus_frontend_state_changed(struct fs_mount *mount, const char *oldstate);
-void xenbus_watch_frontend_state(struct fs_mount *mount);
-void xenbus_unwatch_frontend_state(struct fs_mount *mount);
+bool xenbus_watch_frontend_state(struct fs_mount *mount);
+bool xenbus_unwatch_frontend_state(struct fs_mount *mount);
 char* xenbus_read_frontend_state(struct fs_mount *mount);
 
 /* File operations, implemented in fs-ops.c */

diff --git a/tools/fs-back/fs-ops.c b/tools/fs-back/fs-ops.c
index 420b217b2fe11f148e24f83d1395d5aa1d248a80..2dc9ba61255b5c35f84d1b821b4788843bcdae55 100644 (file)
--- a/tools/fs-back/fs-ops.c
+++ b/tools/fs-back/fs-ops.c
@@ -99,7 +99,10 @@ static void dispatch_file_open(struct fs_mount *mount, struct fsif_request *req)
         }
     }
 out:
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -187,7 +190,11 @@ static void dispatch_file_read(struct fs_mount *mount, struct fsif_request *req)
     priv_req->aiocb.aio_sigevent.sigev_notify = SIGEV_SIGNAL;
     priv_req->aiocb.aio_sigevent.sigev_signo = SIGUSR2;
     priv_req->aiocb.aio_sigevent.sigev_value.sival_ptr = priv_req;
-    assert(aio_read(&priv_req->aiocb) >= 0);
+    if (aio_read(&priv_req->aiocb) < 0) {
+        FS_DEBUG("ERROR: aio_read failed errno=%d\n", errno);
+        xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count);
+        terminate_mount_request(mount);
+    }
 
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
@@ -201,9 +208,10 @@ static void end_file_read(struct fs_mount *mount, struct fs_request *priv_req)
     uint16_t req_id;
 
     /* Release the grant */
-    assert(xc_gnttab_munmap(mount->gnth, 
-                            priv_req->page, 
-                            priv_req->count) == 0);
+    if (xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
 
     /* Get a response from the ring */
     rsp_idx = mount->ring.rsp_prod_pvt++;
@@ -257,7 +265,11 @@ static void dispatch_file_write(struct fs_mount *mount, struct fsif_request *req
     priv_req->aiocb.aio_sigevent.sigev_notify = SIGEV_SIGNAL;
     priv_req->aiocb.aio_sigevent.sigev_signo = SIGUSR2;
     priv_req->aiocb.aio_sigevent.sigev_value.sival_ptr = priv_req;
-    assert(aio_write(&priv_req->aiocb) >= 0);
+    if (aio_write(&priv_req->aiocb) < 0) {
+        FS_DEBUG("ERROR: aio_write failed errno=%d\n", errno);
+        xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count);
+        terminate_mount_request(mount);
+    }
 
      
     /* We can advance the request consumer index, from here on, the request
@@ -272,9 +284,10 @@ static void end_file_write(struct fs_mount *mount, struct fs_request *priv_req)
     uint16_t req_id;
 
     /* Release the grant */
-    assert(xc_gnttab_munmap(mount->gnth, 
-                            priv_req->page, 
-                            priv_req->count) == 0);
+    if (xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     
     /* Get a response from the ring */
     rsp_idx = mount->ring.rsp_prod_pvt++;
@@ -392,7 +405,10 @@ static void dispatch_remove(struct fs_mount *mount, struct fsif_request *req)
         ret = remove(file_name);
     }
     FS_DEBUG("Got ret: %d\n", ret);
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -435,7 +451,10 @@ static void dispatch_rename(struct fs_mount *mount, struct fsif_request *req)
         ret = rename(old_file_name, new_file_name);
     }
     FS_DEBUG("Got ret: %d\n", ret);
-    assert(xc_gnttab_munmap(mount->gnth, buf, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, buf, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -500,7 +519,10 @@ static void dispatch_create(struct fs_mount *mount, struct fsif_request *req)
         }
     }
 out:
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     FS_DEBUG("Got ret %d (errno=%d)\n", ret, errno);
 
     /* Get a response from the ring */
@@ -556,7 +578,8 @@ static void dispatch_list(struct fs_mount *mount, struct fsif_request *req)
     /* If there was any error with reading the directory, errno will be set */
     error_code = errno;
     /* Copy file names of the remaining non-NULL dirents into buf */
-    assert(NAME_MAX < XC_PAGE_SIZE >> 1);
+    if (NAME_MAX >= XC_PAGE_SIZE >> 1)
+        goto error_out;
     while(dirent != NULL && 
             (XC_PAGE_SIZE - ((unsigned long)buf & XC_PAGE_MASK) > NAME_MAX))
     {
@@ -572,8 +595,11 @@ error_out:
     ret_val = ((nr_files << NR_FILES_SHIFT) & NR_FILES_MASK) | 
               ((error_code << ERROR_SHIFT) & ERROR_MASK) | 
               (dirent != NULL ? HAS_MORE_FLAG : 0);
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
-    
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
+
     /* Get a response from the ring */
     rsp_idx = mount->ring.rsp_prod_pvt++;
     FS_DEBUG("Writing response at: idx=%d, id=%d\n", rsp_idx, req_id);
@@ -640,7 +666,10 @@ static void dispatch_fs_space(struct fs_mount *mount, struct fsif_request *req)
     if(ret >= 0)
         ret = stat.f_bsize * stat.f_bfree;
 
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -680,9 +709,11 @@ static void dispatch_file_sync(struct fs_mount *mount, struct fsif_request *req)
     priv_req->aiocb.aio_sigevent.sigev_notify = SIGEV_SIGNAL;
     priv_req->aiocb.aio_sigevent.sigev_signo = SIGUSR2;
     priv_req->aiocb.aio_sigevent.sigev_value.sival_ptr = priv_req;
-    assert(aio_fsync(O_SYNC, &priv_req->aiocb) >= 0);
+    if (aio_fsync(O_SYNC, &priv_req->aiocb) < 0) {
+        FS_DEBUG("ERROR: aio_fsync failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
 
-     
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;

diff --git a/tools/fs-back/fs-xenbus.c b/tools/fs-back/fs-xenbus.c
index cc47dbcfd624e88b645da18d2bdf8fd9552bd0f5..603bfb9ef4d90a15025a9510bfd7d6dd0fa017a3 100644 (file)
--- a/tools/fs-back/fs-xenbus.c
+++ b/tools/fs-back/fs-xenbus.c
@@ -107,11 +107,15 @@ int xenbus_get_watch_fd(void)
     int res;
     assert(xsh != NULL);
     res = xs_watch(xsh, WATCH_NODE, "conn-watch");
-    assert(res);
+    if (!res) {
+        FS_DEBUG("ERROR: xs_watch %s failed ret=%d errno=%d\n",
+                 WATCH_NODE, res, errno);
+        return -1;
+    }
     return xs_fileno(xsh); 
 }
 
-void xenbus_read_mount_request(struct fs_mount *mount, char *frontend)
+int xenbus_read_mount_request(struct fs_mount *mount, char *frontend)
 {
     char node[1024];
     char *s;
@@ -126,12 +130,18 @@ void xenbus_read_mount_request(struct fs_mount *mount, char *frontend)
     mount->frontend = frontend;
     snprintf(node, sizeof(node), "%s/state", frontend);
     s = xs_read(xsh, XBT_NULL, node, NULL);
-    assert(strcmp(s, STATE_READY) == 0);
+    if (strcmp(s, STATE_READY) != 0) {
+        FS_DEBUG("ERROR: frontend not read\n");
+        goto error;
+    }
     free(s);
     snprintf(node, sizeof(node), "%s/ring-size", frontend);
     s = xs_read(xsh, XBT_NULL, node, NULL);
     mount->shared_ring_size = atoi(s);
-    assert(mount->shared_ring_size <= MAX_RING_SIZE);
+    if (mount->shared_ring_size > MAX_RING_SIZE) {
+        FS_DEBUG("ERROR: shared_ring_size (%d) > MAX_RING_SIZE\n", mount->shared_ring_size);
+        goto error;
+    }
     free(s);
     for(i=0; i<mount->shared_ring_size; i++)
     {
@@ -144,6 +154,11 @@ void xenbus_read_mount_request(struct fs_mount *mount, char *frontend)
     s = xs_read(xsh, XBT_NULL, node, NULL);
     mount->remote_evtchn = atoi(s);
     free(s);
+    return 0;
+
+error:
+    free(s);
+    return -1;
 }
 
 /* Small utility function to figure out our domain id */
@@ -161,7 +176,7 @@ static int get_self_id(void)
 } 
 
 
-void xenbus_write_backend_node(struct fs_mount *mount)
+bool xenbus_write_backend_node(struct fs_mount *mount)
 {
     char node[1024], backend_node[1024];
     int self_id;
@@ -175,10 +190,10 @@ void xenbus_write_backend_node(struct fs_mount *mount)
     xs_write(xsh, XBT_NULL, node, backend_node, strlen(backend_node));
 
     snprintf(node, sizeof(node), ROOT_NODE"/%d/state", mount->mount_id);
-    xs_write(xsh, XBT_NULL, node, STATE_INITIALISED, strlen(STATE_INITIALISED));
+    return xs_write(xsh, XBT_NULL, node, STATE_INITIALISED, strlen(STATE_INITIALISED));
 }
 
-void xenbus_write_backend_state(struct fs_mount *mount, const char *state)
+bool xenbus_write_backend_state(struct fs_mount *mount, const char *state)
 {
     char node[1024];
     int self_id;
@@ -186,29 +201,25 @@ void xenbus_write_backend_state(struct fs_mount *mount, const char *state)
     assert(xsh != NULL);
     self_id = get_self_id();
     snprintf(node, sizeof(node), ROOT_NODE"/%d/state", mount->mount_id);
-    xs_write(xsh, XBT_NULL, node, state, strlen(state));
+    return xs_write(xsh, XBT_NULL, node, state, strlen(state));
 }
 
-void xenbus_watch_frontend_state(struct fs_mount *mount)
+bool xenbus_watch_frontend_state(struct fs_mount *mount)
 {
-    int res;
     char statepath[1024];
 
     assert(xsh != NULL);
     snprintf(statepath, sizeof(statepath), "%s/state", mount->frontend);
-    res = xs_watch(xsh, statepath, "frontend-state");
-    assert(res);
+    return xs_watch(xsh, statepath, "frontend-state");
 }
 
-void xenbus_unwatch_frontend_state(struct fs_mount *mount)
+bool xenbus_unwatch_frontend_state(struct fs_mount *mount)
 {
-    int res;
     char statepath[1024];
 
     assert(xsh != NULL);
     snprintf(statepath, sizeof(statepath), "%s/state", mount->frontend);
-    res = xs_unwatch(xsh, statepath, "frontend-state");
-    assert(res);
+    return xs_unwatch(xsh, statepath, "frontend-state");
 }
 
 int xenbus_frontend_state_changed(struct fs_mount *mount, const char *oldstate)